[HLSL] Fix resource kind for RasterizerOrderedStructuredBuffer #116700
Conversation
This is a kind of StructuredBuffer, so it should be "Raw" and not "Typed".
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-hlsl Author: Justin Bogner (bogner) ChangesThis is a kind of StructuredBuffer, so it should be "Raw" and not "Typed". Full diff: https://github.com/llvm/llvm-project/pull/116700.diff 2 Files Affected:
diff --git a/clang/lib/Sema/HLSLExternalSemaSource.cpp b/clang/lib/Sema/HLSLExternalSemaSource.cpp
index cac15b974a276e..1013885928b1d8 100644
--- a/clang/lib/Sema/HLSLExternalSemaSource.cpp
+++ b/clang/lib/Sema/HLSLExternalSemaSource.cpp
@@ -548,7 +548,7 @@ void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
.Record;
onCompletion(Decl, [this](CXXRecordDecl *Decl) {
setupBufferType(Decl, *SemaPtr, ResourceClass::UAV,
- ResourceKind::TypedBuffer, /*IsROV=*/true,
+ ResourceKind::RawBuffer, /*IsROV=*/true,
/*RawBuffer=*/true)
.addArraySubscriptOperators()
.completeDefinition();
diff --git a/clang/test/AST/HLSL/RasterizerOrderedStructuredBuffer-AST.hlsl b/clang/test/AST/HLSL/RasterizerOrderedStructuredBuffer-AST.hlsl
index f334e1bb6db3fc..eb8bfeffb481f1 100644
--- a/clang/test/AST/HLSL/RasterizerOrderedStructuredBuffer-AST.hlsl
+++ b/clang/test/AST/HLSL/RasterizerOrderedStructuredBuffer-AST.hlsl
@@ -35,7 +35,7 @@ RasterizerOrderedStructuredBuffer<int> Buffer;
// CHECK-SAME{LITERAL}: [[hlsl::is_rov]]
// CHECK-SAME{LITERAL}: [[hlsl::raw_buffer]]
// CHECK-SAME{LITERAL}: [[hlsl::contained_type(element_type)]]
-// CHECK-NEXT: HLSLResourceAttr 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> Implicit TypedBuffer
+// CHECK-NEXT: HLSLResourceAttr 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> Implicit RawBuffer
// CHECK: CXXMethodDecl 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> <invalid sloc> operator[] 'element_type &const (unsigned int) const'
// CHECK-NEXT: ParmVarDecl 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> <invalid sloc> Idx 'unsigned int'
@@ -63,4 +63,4 @@ RasterizerOrderedStructuredBuffer<int> Buffer;
// CHECK-SAME{LITERAL}: [[hlsl::is_rov]]
// CHECK-SAME{LITERAL}: [[hlsl::raw_buffer]]
// CHECK-SAME{LITERAL}: [[hlsl::contained_type(int)]]
-// CHECK-NEXT: HLSLResourceAttr 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> Implicit TypedBuffer
+// CHECK-NEXT: HLSLResourceAttr 0x{{[0-9A-Fa-f]+}} <<invalid sloc>> Implicit RawBuffer
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
| ResourceKind::TypedBuffer, | ||
| /*IsROV=*/false, /*RawBuffer=*/false) | ||
| ResourceKind::TypedBuffer, /*IsROV=*/false, | ||
| /*RawBuffer=*/false) |
There was a problem hiding this comment.
Why was this reformatted? I think there's no change here other than formatting - I thought we had something that enforced clang-format conformance?
There was a problem hiding this comment.
clang-format was complaining about the line I actually changed and I noticed that all of these looked funny. Seems like there's a "comment at the beginning of a line" heuristic that stops clang-format from messing with these, but if you collapse them to a single line and then run clang-format it does something pretty reasonable.
| setupBufferType(Decl, *SemaPtr, ResourceClass::UAV, ResourceKind::RawBuffer, | ||
| /*IsROV=*/true, /*RawBuffer=*/true) |
There was a problem hiding this comment.
When would we ever want RawBuffer to be false and ResourceKind to be something other than RawBuffer? Or are we going to be removing one of these in the future?
There was a problem hiding this comment.
ResourceKind won't be specified here in the future (See #104862), which will remove the redundancy. This change doesn't actually do much as is, but the inconsistency was confusing.
4 participants
This is a kind of StructuredBuffer, so it should be "Raw" and not "Typed".